-
-
Notifications
You must be signed in to change notification settings - Fork 724
feat(codegen): preserve comments between CatchClause's param and body #16167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
CodSpeed Performance ReportMerging #16167 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for preserving comments between a catch clause's parameter and body, enabling code coverage tools that use ignore hints (like /* v8 ignore next */) to properly instrument catch blocks.
Key Changes:
- Modified comment parsing to treat line comments after
)as leading rather than trailing - Added comment printing logic in
CatchClausecodegen before the block body - Added manual handling of the
print_next_indent_as_spaceflag to ensure proper spacing
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/oxc_parser/src/lexer/trivia_builder.rs | Updated trailing comment detection to exclude Kind::RParen, making line comments after ) be treated as leading comments attached to the next token |
| crates/oxc_codegen/src/gen.rs | Added print_comments_at(self.body.span.start) before printing catch block body, with manual handling of spacing flag for inline comments |
| crates/oxc_codegen/tests/integration/comments.rs | Added three test cases covering inline block comments, multiple comments, and line comments between catch parameter and body |
| crates/oxc_codegen/tests/integration/snapshots/coverage.snap | Added snapshot expectations for the three new test cases showing proper comment preservation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: sapphi-red <[email protected]>
Co-authored-by: sapphi-red <[email protected]>
Co-authored-by: sapphi-red <[email protected]>
Co-authored-by: sapphi-red <[email protected]>
…tch clause support Co-authored-by: sapphi-red <[email protected]>
… documentation Co-authored-by: sapphi-red <[email protected]>
78c730b to
bc81ea4
Compare
crates/oxc_codegen/src/gen.rs
Outdated
| p.print_soft_space(); | ||
| p.print_comments_at(self.body.span.start); | ||
| // Consume the space flag set by comment printing to ensure proper spacing before the opening brace | ||
| if p.print_next_indent_as_space { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot add !p.options.minify and add tests for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added !p.options.minify guard to the space handling logic and added tests for both minify mode with comments enabled (preserves comments) and true minify mode with comments disabled (strips comments). See 4674fd9.
… and add tests Co-authored-by: sapphi-red <[email protected]>
CatchClause'sparamandbodyCatchClausegen incrates/oxc_codegen/src/gen.rsprint_comments_at(self.body.span.start)before printing the block statement, and consume the pending space flag!p.options.minifyguard to space handling and add minify testOriginal prompt
CatchClause'sparamandbody#16076✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.